-
Notifications
You must be signed in to change notification settings - Fork 469
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Removes ethereumjs-util and keccak dependencies. #357
Conversation
Is there anything I can do to expedite this getting merged? |
I believe the coveralls coverage decrease is because I deleted some lines that were previously covered, so the number of covered lines as a percentage went down (uncovered lines have more weight as of this change). |
Sorry for the delay. I see a reason to not use two packages for keccak256, but what's wrong with ethereumjs-util? It is supposed to be well maintained. |
|
I'd prefer depending on |
Keccack256 is called in two places, nowhere near a hot path from what I can tell. Given the many orders of magnitude slower the compilation process is than hashing, I think choosing the native library for its performance benefits at the cost of increasing dev complexity is very much not worth it. |
Replaces both with `js-sha3`. `ethereumjs-util` and `keccak` NPM packages both have native bindings in their dependency tree which means they depend on node-gyp along the way. `js-sha3` on the other hand is a native implementation of the sha3 family of hashing functions, which relieves a number of `node-gyp` and native binding related issues.
The use in The use in |
Replaces both with
js-sha3
.ethereumjs-util
andkeccak
NPM packages both have native bindings in their dependency tree which means they depend on node-gyp along the way.js-sha3
on the other hand is a JavaScript-native implementation of the sha3 family of hashing functions, which relieves a number ofnode-gyp
and native binding related issues.Fixes #356
Fixes #336
Part of #361